REF: ArrowEA _data->_pa_array #50987
Conversation
mroeschke
added
Refactor
|
@mroeschke if you're not on board we can close this, otherwise ill rebase |
|
Curious, would this make it harder for |
It would if we did that, but they don't share any code... Also the ._data attributes are different types, which i would think is exactly the kind of thing its nice to have clear names for |
|
I was thinking of, for example, utilizing the base masked engine work in the short term to have better indexing support which I think utilizes But overall it's not that big of a deal, I am fine changing it to |
|
@mroeschke any objection to merging before more merge conflicts pile up? |
yep. Go ahead and change if you feel it's an improvement. |
|
no objections but also no enthusiasm. im on the fence about self-merging. |
|
Just fixed another merge conflict. im planning to merge on green unless someone objects. |
|
Thanks @jbrockmendel |
| return state | ||
|
|
||
| def __setstate__(self, state) -> None: | ||
| state["_data"] = pa.chunked_array(state["_data"]) | ||
| state["_pa_array"] = pa.chunked_array(state["_data"]) |
There was a problem hiding this comment.
@jbrockmendel I guess not changing the _data key here and above as well was done because of backwards compatibility? This caused a bug, see dask/dask#10036
There was a problem hiding this comment.
yah, looks like we'll need to check for both _data and _pa_array until older pickles are dropped
Totally subjective: I prefer a distinctive/informative name. This makes it easy to e.g. grep for all the places where we access this attribute.